Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Images in labels #15937

Merged
merged 16 commits into from
Dec 2, 2019
Merged

[core] Images in labels #15937

merged 16 commits into from
Dec 2, 2019

Conversation

alexshalamov
Copy link
Contributor

@alexshalamov alexshalamov commented Nov 18, 2019

Images in labels (text-field)

This PR is an implementation of a proposal in mapbox/mapbox-gl-js#8604.

Style interface

Images could be added to a label via image expression. For example:

"text-field": ["format", "London ", ["image", "london-underground"]]

Implementation details

  • Internally, image sections are represented as unicode points from Basic Multilingual Plane Unicode Private Use Area (PUA), so that image positions / style can be preserved after BiDi and/or shaping algorithms are run.
  • Quad vertices have a flag indicating whether quad is an sdf image
  • Labels that have text and images, are drawn with the new program that has a branch in a fragment shader. Based on an 'sdf' flag value, the branch uses an image atlas to draw a texture or a glyph atlas to draw a text glyph (sdf image)
  • SDF images in labels are not supported (could be implemented in follow-up PRs, if needed)

Possible follow-up improvements

  • To preserve image size for zoom-dependent text-size values (fractional zoom levels), we could try storing layoutTextSize that was used during layout and pass it as a uniform when label is rendered. Then, image could be scaled according to the size that is interpolated in a shader.
  • image-scale option might be added to a formatted image section, so that designers can have more control over the size of an image.

Tests

Expression and render tests are in mapbox/mapbox-gl-js#8904, unit tests are in this PR.

@alexshalamov alexshalamov force-pushed the alexshalamov_images_in_labels branch 6 times, most recently from 9382da7 to fa76f9a Compare November 18, 2019 12:17
@alexshalamov alexshalamov changed the title [core] Images in label [core] Images in labels Nov 18, 2019
@alexshalamov alexshalamov force-pushed the alexshalamov_images_in_labels branch 2 times, most recently from f9a2f06 to 9951eba Compare November 18, 2019 13:50
@alexshalamov alexshalamov marked this pull request as ready for review November 18, 2019 14:54
@alexshalamov alexshalamov self-assigned this Nov 18, 2019
@alexshalamov alexshalamov added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Nov 18, 2019
@alexshalamov alexshalamov force-pushed the alexshalamov_images_in_labels branch 2 times, most recently from 7c5e0a4 to d9790a5 Compare November 20, 2019 13:13
@alexshalamov alexshalamov removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Nov 20, 2019
scripts/generate-shaders.js Outdated Show resolved Hide resolved
src/mbgl/layout/symbol_layout.cpp Show resolved Hide resolved
include/mbgl/style/expression/formatted.hpp Show resolved Hide resolved
src/mbgl/text/tagged_string.cpp Show resolved Hide resolved
@chloekraw chloekraw added the needs changelog Indicates PR needs a changelog entry prior to merging. label Nov 20, 2019
@alexshalamov alexshalamov force-pushed the alexshalamov_images_in_labels branch 3 times, most recently from 3dcd06c to 2032a10 Compare November 25, 2019 09:38
@alexshalamov alexshalamov requested a review from 1ec5 as a code owner November 25, 2019 09:38
@alexshalamov alexshalamov requested a review from a team November 25, 2019 09:38
@alexshalamov alexshalamov removed the needs changelog Indicates PR needs a changelog entry prior to merging. label Nov 25, 2019
Copy link
Contributor

@pozdnyakov pozdnyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall!

src/mbgl/text/tagged_string.hpp Outdated Show resolved Hide resolved
src/mbgl/style/expression/formatted.cpp Show resolved Hide resolved
src/mbgl/text/tagged_string.hpp Outdated Show resolved Hide resolved
src/mbgl/text/tagged_string.cpp Show resolved Hide resolved
src/mbgl/text/tagged_string.cpp Outdated Show resolved Hide resolved
src/mbgl/text/tagged_string.cpp Outdated Show resolved Hide resolved
src/mbgl/text/shaping.cpp Outdated Show resolved Hide resolved
@alexshalamov
Copy link
Contributor Author

Changelog entry

  • Added support for embedding images into labels in core library. Runtime APIs for the feature will be implemented separately. (#15937)

@alexshalamov alexshalamov force-pushed the alexshalamov_images_in_labels branch from 2032a10 to acf1013 Compare November 28, 2019 09:14
@alexshalamov
Copy link
Contributor Author

@tmpsantos File size does not match at probe "linux-clang8-mbgl-offline": 5597912, expected is 5532376.

65536 diff looks suspicious, I think we need to use something like bloaty to get precise size of a binary.

@alexshalamov alexshalamov force-pushed the alexshalamov_images_in_labels branch 4 times, most recently from c3fdec2 to dfeb7d7 Compare November 28, 2019 20:24
@alexshalamov alexshalamov force-pushed the alexshalamov_images_in_labels branch from dfeb7d7 to caee9ce Compare December 2, 2019 10:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants